Skip to content

Conversation

@Teagan42
Copy link
Contributor

What

  • wrap PlexServer's FastMCP lifespan hook in an asynccontextmanager so cleanup runs under HTTP transports
  • add a regression test that exercises the low-level server lifespan to guard against regressions
  • bump the package version to 0.26.26 and refresh uv.lock

Why

  • SSE connections were crashing because FastMCP attempted to enter a plain async generator instead of an async context manager

Affects

  • FastMCP server lifecycle handling and the test suite

Testing

  • uv run ruff check .
  • uv run pytest

Documentation

  • n/a

https://chatgpt.com/codex/tasks/task_e_68d27e285d44832880c41fef310052b2

Copilot AI review requested due to automatic review settings September 23, 2025 11:21
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a FastMCP lifespan issue where SSE connections were crashing due to improper async context manager usage. The fix wraps the PlexServer's lifespan hook in an asynccontextmanager decorator to ensure proper cleanup under HTTP transports.

  • Converts the plain async generator to an async context manager with try/finally structure
  • Adds a regression test to verify the server lifespan functionality works correctly
  • Updates package version to 0.26.26

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

File Description
mcp_plex/server.py Adds asynccontextmanager decorator and try/finally structure to lifespan hook
tests/test_server.py Adds regression test for server lifespan context manager functionality
pyproject.toml Bumps version from 0.26.25 to 0.26.26

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +61 to +64
try:
yield
finally:
await self.close()
Copy link

Copilot AI Sep 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The try/finally block is redundant here. Since asynccontextmanager already handles exceptions and ensures cleanup code runs, the bare try/yield/finally pattern doesn't add value and can be simplified to just yield followed by await self.close().

Suggested change
try:
yield
finally:
await self.close()
yield
await self.close()

Copilot uses AI. Check for mistakes.
@github-actions
Copy link

Coverage

Coverage Report
FileStmtsMissCoverMissing
mcp_plex
   loader.py3862494%95, 124–128, 130–139, 150, 152–154, 190, 592–594
   server.py3012093%106–107, 230, 458, 462, 484–490, 522, 532, 563–564, 581–582, 605, 608–609, 667
TOTAL8684495% 

Tests Skipped Failures Errors Time
60 0 💤 0 ❌ 0 🔥 30.903s ⏱️

@Teagan42 Teagan42 merged commit 0166029 into main Sep 23, 2025
4 checks passed
@Teagan42 Teagan42 deleted the codex/fix-asgi-application-connection-error branch September 23, 2025 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants